feat(27-02): map loop failures to typed FailureState records - #21
Conversation
Loop boundary failures now produce typed FailureState evidence (CAS Contracts schema) instead of escaping uncaught. Includes FailureClassifier wiring in LoopCoordinator and unit coverage. Rescued 2026-07-08: implementation was verified in the 27-02 session but never committed (caught by the Phase 35 audit's live spot-verification). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f61824f84
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await store.SaveAsync(aggregate, cancellationToken); | ||
|
|
||
| var evidence = aggregate.Evidence.Select(item => item.Uri).Distinct(StringComparer.Ordinal).ToArray(); | ||
| await learning.PublishAsync(new(goalId, aggregate.Goal.CorrelationId, outcome, $"Goal {outcome} after {workerAttempts} worker attempt(s).", evidence), cancellationToken); |
There was a problem hiding this comment.
Keep terminal publishing failures from rewriting outcomes
When learning.PublishAsync throws after store.SaveAsync has already persisted a completed outcome, this call is still inside the broad try; the catch reloads that terminal aggregate and calls CompleteWithFailure, so a telemetry/MCP publication outage rewrites a successful goal/work item to Failed with a synthetic failure-state. Keep terminal publication failures outside the failure-mapping path or avoid overwriting already-terminal outcomes.
Useful? React with 👍 / 👎.
| Leases = aggregate.Leases.Where(item => item.Id != leaseId).ToArray(), | ||
| BudgetReservations = [], | ||
| Evidence = [.. aggregate.Evidence, new EvidenceRecord(Guid.NewGuid().ToString("N"), aggregate.Goal.Id, workItemId, "failure-state", evidenceUri)], | ||
| Transitions = [.. aggregate.Transitions, new(Guid.NewGuid().ToString("N"), aggregate.Goal.Id, aggregate.Goal.Status.ToString(), status.ToString(), status == GoalStatus.Cancelled ? GoalStopReason.Cancellation.ToString() : GoalStopReason.UnrecoverableFault.ToString(), now)], |
There was a problem hiding this comment.
Preserve class-specific stop reasons for typed failures
For any non-cancellation typed failure, including UnauthorizedAccessException classified as Policy and TimeoutException classified as retryable Transient, this transition is recorded as UnrecoverableFault. That makes consumers of the transition stream unable to distinguish policy denials or retryable transient failures from actual unrecoverable faults despite the new FailureState classification; map the FailureClass to the corresponding stop reason/action instead of collapsing everything to unrecoverable.
Useful? React with 👍 / 👎.
Summary
FailureStaterecords (CAS Contracts schema) withFailureClassifierwiring inLoopCoordinator, instead of escaping uncaughtFailureStateTests.cs+ extendedLoopCoordinatorTests.csProvenance
Implementation was completed and verified in the Phase 27-02 session (2026-07-07) but never committed — caught by the Phase 35 milestone audit's live spot-verification (REQ-1.4.2 blocker finding #1). Rescued per workspace guardrails; second rescue of this class after #20.
Merge order
Land after #16 (coverage gates) and #20 (fault injection) — shares
LoopCoordinatorsurface; rungh pr update-branchif conflicts.🤖 Generated with Claude Code